Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP][SYSTEMDS-3259] Shampoo optimizer #2071

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

OlgaOvcharenko
Copy link
Contributor

@OlgaOvcharenko OlgaOvcharenko commented Aug 22, 2024

This PR adds Shampoo optimizer (matrix case, same to Algorithm 1).

Copy link

codecov bot commented Aug 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.44%. Comparing base (34f05fe) to head (68eab0b).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2071   +/-   ##
=========================================
  Coverage     69.44%   69.44%           
- Complexity    41496    41501    +5     
=========================================
  Files          1446     1446           
  Lines        162734   162734           
  Branches      31635    31635           
=========================================
+ Hits         113008   113012    +4     
+ Misses        40738    40736    -2     
+ Partials       8988     8986    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mboehm7
Copy link
Contributor

mboehm7 commented Aug 24, 2024

Thanks @OlgaOvcharenko for getting started on this new optimizer. I would recommend to return L and R as well from the update function (similar to we do it in Adam), add the init function (can be zero initialized), and maybe add the reference to the function documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants